Skip to content

Reject non-list security_opt in service definitions - #1526

Open
Sanjays2402 wants to merge 2 commits into
containers:mainfrom
Sanjays2402:fix/security-opt-must-be-list-1444
Open

Reject non-list security_opt in service definitions#1526
Sanjays2402 wants to merge 2 commits into
containers:mainfrom
Sanjays2402:fix/security-opt-must-be-list-1444

Conversation

@Sanjays2402

Copy link
Copy Markdown

Closes #1444

The Compose Specification defines security_opt as a sequence, and docker compose rejects a bare scalar with services.<service>.security_opt must be a array. podman-compose instead coerced a plain string into a one-element list alongside env_file and volumes, so a mistyped security_opt: label=disable was silently accepted while the equivalent docker compose file fails. normalize_service() now raises PodmanComposeError when the value is not a list.

Compose spec: https://github.com/compose-spec/compose-spec/blob/main/spec.md#security_opt

Contributor Checklist:

Please make sure to read development guidelines in CONTRIBUTING.md. Pull requests that do not
follow the guidelines WILL TAKE LONGER TO REVIEW as the first review comment will be to follow
these guidelines.

  • Read, and the change follows them.

If this PR adds a new feature that improves compatibility with docker-compose, please add a link
to the exact part of compose spec that the PR touches.

  • Linked above (security_opt in the Compose Specification).

For any user-visible change please add a release note to newsfragments directory, e.g.
newsfragments/my_feature.feature. See newsfragments/README.txt for more details.

  • Not included in this PR — happy to add newsfragments/reject_non_list_security_opt.bugfix with the text "Reject non-list security_opt in service definitions instead of silently accepting a bare scalar." on request.

All changes require additional unit tests.

  • Added test_security_opt_must_be_a_list (parametrized over a bare string and a mapping) and test_security_opt_list_is_normalized to tests/unit/test_normalize_service.py. Both new cases fail without the change and pass with it; tests/unit is 470 passed.

This change was prepared with AI assistance; the regression test was run locally and fails without the fix.

The Compose Specification defines security_opt as a sequence, and
docker compose rejects a bare scalar with "services.<service>.security_opt
must be a array". podman-compose instead wrapped a plain string into a
one-element list alongside env_file and volumes, so a mistyped

    security_opt: label=disable

was silently accepted while the equivalent docker compose file fails.

Drop security_opt from the string-to-list coercion and raise
PodmanComposeError when the value is not a list.

Closes containers#1444

@p12tic p12tic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add release note in newsfragments/. Otherwise looks good, thanks!

@Sanjays2402

Copy link
Copy Markdown
Author

Added in 19da98b as newsfragments/reject_non_list_security_opt.bugfix, following the .bugfix towncrier type from newsfragments/README.txt:

Reject a non-list security_opt in service definitions instead of silently coercing a bare scalar into a one-element list.

tests/unit/test_normalize_service.py is 28 passed on the new head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security_opt allows value without a dash

2 participants